Verdict: REQUEST CHANGES

Summary

The custom Farcaster connector is a reasonable direction, but the manual connect flow now regresses standalone browser behavior.

Findings

• [high] ConnectWallet always prefers the Farcaster connector on button click whenever that connector exists in wagmi config. Because lib/wagmi.ts always registers farcaster() first, a standalone browser session will try the Farcaster connector before injected(), which can fail outside a real Farcaster mini app and breaks issue #37's requirement to keep existing wallet connection working outside Farcaster.
  • File: src/components/ConnectWallet.tsx:49
  • Suggestion: only select the Farcaster connector when you have confirmed Farcaster mini app context, otherwise keep the standalone path on injected().

Decision

Requesting changes because the current button handler can route non-Farcaster users through the wrong connector.

Review: REQUEST CHANGES

Must fix

1. (High) Manual connect always prefers Farcaster connector, even outside Farcaster
ConnectWallet.tsx lines 48-54 — connectors.find((c) => c.type === "farcaster") will always succeed because the connector is unconditionally registered in wagmi.ts. The ?? fallback to injected is dead code. Outside Farcaster, clicking Connect will call sdk.wallet.getEthereumProvider() which will throw or return a non-functional provider.

Fix: gate on isAuthorized() or isFarcasterMiniApp() before selecting the Farcaster connector:

onClick={async () => {
  const fc = connectors.find((c) => c.type === "farcaster");
  const inj = connectors.find((c) => c.type === "injected");
  const useFarcaster = fc && await fc.isAuthorized();
  connect({ connector: useFarcaster ? fc : (inj ?? fc)! });
}}

Should fix

2. (Medium) No error handling in resolveProvider() propagation path
resolveProvider() can throw if the SDK fails to initialize. The auto-connect path is protected (via isAuthorized() try/catch), but the manual connect path is not. Fixing #1 also mitigates this.

3. (Low) farcaster.type assigned before function declaration
Line 38 assigns farcaster.type before the farcaster() function declaration on line 44. Works due to hoisting but is confusing — move it after the declaration.

Should verify

4. Was @farcaster/miniapp-wagmi-connector removed from package.json? The diff doesn't include package.json changes. If the incompatible package was previously installed, remove it.

Looks good

• Wagmi v3 createConnector API: all required methods implemented correctly
• Provider caching via module-level variable — avoids recreation
• Lazy dynamic import of @farcaster/miniapp-sdk — good for bundle size and SSR
• isFarcasterMiniApp() detection with SSR guard and try/catch
• Auto-connect via useEffect with useRef guard — clean pattern
• disconnect() no-op is correct for Farcaster embedded wallet
• switchChain implementation with hex conversion and emitter — correct

T2b Review: REQUEST CHANGES

Custom wagmi v3 connector is well-implemented — createConnector API fully covered, provider caching and lazy SDK import are clean, auto-connect with useRef guard is good.

Blocking

1. Manual connect broken outside Farcaster — ConnectWallet.tsx onClick handler unconditionally picks the Farcaster connector because it's always registered in the wagmi config. The ?? injected fallback is dead code. Must gate connector selection on isAuthorized() or isFarcasterMiniApp() before choosing — otherwise users outside Farcaster can't connect their wallet.

Medium

2. Unhandled errors on manual connect path — resolveProvider() can throw if SDK isn't available. Auto-connect path is protected by isAuthorized(), but manual connect has no try/catch.

Verify

3. Was @farcaster/miniapp-wagmi-connector removed from package.json? — No package.json change in the diff. If it was installed in PR #146, it should be removed now that the custom connector replaces it.

T2b Re-review: APPROVE

Blocking issue fixed — connector selection properly gated by isFarcasterMiniApp() state. Manual connect uses Farcaster connector only when inMiniApp is true, falls back to injected otherwise. Auto-connect also gated. Clean implementation.

Verdict: APPROVE

Summary

The Farcaster wallet integration now preserves the standalone injected wallet flow while enabling Farcaster auto-detection and embedded wallet usage inside mini app context.

Findings

• [info] No blocking findings.

Decision

Approving because the connector selection is now correctly gated on confirmed Farcaster mini app context and lint-and-typecheck passed.